Repository navigation
feat(database): support creating database configs from a URL - #2315
Conversation
Benchmark ResultsComparison of Open to see the benchmark resultsNo benchmark changes above ±5%. Generated by phpbench against commit b58ea88 |
aidan-casey
left a comment
aidan-casey
left a comment
There was a problem hiding this comment.
Hey, @boydbloemsma! Thanks for taking this on, I appreciate your work on this.
First, I would recommend we rename this to DatabaseConnectionString, which is the term typically used for this string.
Second (and this is technically a limitation of the existing implementation as well), we need to allow for query parameters within the DSN such as connect_timeout, keepalives, sslmode, etc.
Third, I'd like to see a test case with a Unix socket in the DSN added to this.
Finally, I'd suggest we make this a true value object with methods such as getHost, etc. I'm not sure we should perform the actual validation on the scheme within the VO vs. the config itself.
|
Agree with Aidan's feedback expect for the getters. Don't add getters but use public readable properties |
|
Thanks for the feedback guys! |
|
Appreciate it! Nice feature :) |
|
Nice! Thanks, @boydbloemsma! |
Fixes #2310